Skip to content

Conversation

@gunli
Copy link
Contributor

@gunli gunli commented Mar 7, 2025

Fixes #

Master Issue: #1335 #1336

Motivation

Reading is run in a seperate goroutine, connection.incomingCmdCh is redundant, just call connection.internalReceivedCommand() directly.

func (c *connection) run() {
	pingSendTicker := time.NewTicker(c.keepAliveInterval)
	pingCheckTicker := time.NewTicker(c.keepAliveInterval)

	defer func() {
		// stop tickers
		pingSendTicker.Stop()
		pingCheckTicker.Stop()

		// all the accesses to the pendingReqs should be happened in this run loop thread,
		// including the final cleanup, to avoid the issue
		// https://github.com/apache/pulsar-client-go/issues/239
		c.failPendingRequests(errConnectionClosed)
		c.Close()
	}()

	// All reads come from the reader goroutine
	go c.reader.readFromConnection()  // reading is run in a SEPERATE goroutine here
	go c.runPingCheck(pingCheckTicker)
}

Modifications

delete redundant channel connection.incomingCmdCh

Verifying this change

  • Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@gunli gunli mentioned this pull request Mar 7, 2025
1 task
@gunli
Copy link
Contributor Author

gunli commented Mar 7, 2025

Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodece
Copy link
Member

nodece commented Mar 7, 2025

In the Golang, we must use the separate goroutine to perform the read and write operation.

@BewareMyPower BewareMyPower added this to the v0.15.0 milestone Mar 7, 2025
Copy link
Member

@crossoverJie crossoverJie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nodece nodece merged commit 6cb2867 into apache:master Mar 7, 2025
7 checks passed
gunli added a commit to gunli/pulsar-client-go that referenced this pull request Mar 7, 2025
crossoverJie pushed a commit that referenced this pull request Mar 7, 2025
* rebase to master to merge #1343

* make consumer is not nil

* revert test case

* revert test case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants